Fix panic on blockquote #6684
Fix panic on blockquote #6684
Conversation
shulaoda
force-pushed
the
10-09-fix-issue-6660
branch
from
fa5abb7 to
608078c
Compare
src/comment.rs
Outdated
| // of the new line_start. We update the indent because it effects the max width | ||
| // of each formatted line. | ||
| line_start = itemized_block_quote_start(line, line_start, 2); | ||
| line_start = itemized_block_quote_start(line.trim_start(), line_start, 1); |
There was a problem hiding this comment.
Changed to line.trim_start() because the previous implementation could cause the quote level calculation
let quote_level = line
.chars()
.take_while(|&c| matches!(c, '>' | ' '))
.filter(|&c| c == '>')
.count();to include special whitespace characters (e.g., '\x09') in the prefix.
There was a problem hiding this comment.
I think this implementation is actually slightly incorrect.
A tab is treated as four space characters, which cannot start a blockquote.
A nested blockquote can have four spaces between them:
> > is valid, but > > is not.
A blockquote can start with up to three spaces:
> is fine to start a blockquote, but > will cause it to be treated as a code block.
There was a problem hiding this comment.
You have not addressed my comment: please parse blockquotes according to the spec
There was a problem hiding this comment.
You have not addressed my comment: please parse blockquotes according to the spec
Makes sense — I’ll fix this in this PR.
There was a problem hiding this comment.
However, this might break some of the existing styling and cause visual differences. I’m not sure if it should be addressed in this PR, but I’ll try to fix it for now anyway.
There was a problem hiding this comment.
break some of the existing styling
isn't this an unstable formatter option anyways?
There was a problem hiding this comment.
break some of the existing styling
isn't this an unstable formatter option anyways?
Right, that makes sense — nothing to worry about then!
There was a problem hiding this comment.
@fee1-dead I haven’t handled the second case yet — > is fine to start a blockquote, but > will cause it to be treated as a code block.
This can be determined correctly when the previous line isn’t a list item (-, *, +, 1., or 1)), but issues arise in cases like the one below, since the > actually belongs to the -, and its indentation is determined by the list item. There are many such combinations, and for now, I’ve only handled the > > case.
- aaa
6666
> bbbThere was a problem hiding this comment.
Considering all the cases would be too complex and require significant refactoring. For now, can we focus on fixing the blockquote panic issue only?
src/comment.rs
Outdated
| // of the new line_start. We update the indent because it effects the max width | ||
| // of each formatted line. | ||
| line_start = itemized_block_quote_start(line, line_start, 2); | ||
| line_start = itemized_block_quote_start(line.trim_start(), line_start, 1); |
There was a problem hiding this comment.
I think this implementation is actually slightly incorrect.
A tab is treated as four space characters, which cannot start a blockquote.
A nested blockquote can have four spaces between them:
> > is valid, but > > is not.
A blockquote can start with up to three spaces:
> is fine to start a blockquote, but > will cause it to be treated as a code block.
shulaoda
force-pushed
the
10-09-fix-issue-6660
branch
from
608078c to
79304a8
Compare
Co-authored-by: beef <ent3rm4n@gmail.com>
shulaoda
force-pushed
the
10-09-fix-issue-6660
branch
from
99a8a9d to
a443182
Compare
4 participants
closes #6660, #6586, #6600
Previously, when formatting comments containing blockquotes followed by comparison operators like
> >= >> >>= >>> >>>=, the code would panic withbyte index out of boundserror.The issue was in
itemized_block_quote_startfunction, which counted all>characters in non-alphanumeric prefix, treating operators like>=as nested blockquote markers. This caused the calculated indent to exceed the line length, leading to out-of-bounds indexing.Fixed by only recognizing valid Markdown blockquote syntax where each>must be followed by a space (e.g.,> > text), which was the original intended behavior. Note that only the standard format with spaces is supported, alternative formats like>> text,> > text(multiple spaces), or>text(no space) are not recognized as blockquote markers.